Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(bug): Allowed pending jobs to run on start if pendingJobsInitia… #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thoven87
Copy link
Contributor

Allow pending job to run on start if

pendingJobsInitialization = .rerun

@Joannis
Copy link
Member

Joannis commented Dec 19, 2024

@adam-fowler I'm not sure what expected behaviour would be here.

@adam-fowler
Copy link
Member

If they are flagged as pending they are already in the queue, so this should be unnecessary and might even cause issues. @thoven87 Is this change to fix an issue you are seeing

@thoven87
Copy link
Contributor Author

If they are flagged as pending they are already in the queue, so this should be unnecessary and might even cause issues. @thoven87 Is this change to fix an issue you are seeing

I don't see the potential issue this could cause if when a job gets picked up, it's locked by it's worker.

Another issue I see after running countless of tests is with the onMinutes schedule. (which is off topic from this PR) Say you have at least two jobs with the exact same schedule. Only the first job will get run the second job will say it's next schedule if for the current running schedule and will keep pushing jobs with pending states. Since, these jobs are not registered in the JobController, they are registered in scheduled job controller, they will keep queueing and stay there. I will explain more in Discord. I know @Joannis had some strong options when I first bought this potential issue. I would love to hear some of your thoughts. The way I have some this issue in past, is by introducing a small random delay with each scheduled task before triggering. (I will open a ticket separately for this issue)

@adam-fowler I'm not sure what expected behaviour would be here.

Job with pending states should be run if pendingJobInitialization is set to re-run the Redis implementation has no such limit. cc @adam-fowler

@thoven87
Copy link
Contributor Author

if pendingJobsInitialization = .rerun is unnecessary, should be removed from the configuration options?

@adam-fowler
Copy link
Member

if pendingJobsInitialization = .rerun is unnecessary, should be removed from the configuration options?

I guess we could add another enum, that just has cases doNothing, remove

@thoven87
Copy link
Contributor Author

if pendingJobsInitialization = .rerun is unnecessary, should be removed from the configuration options?

I guess we could add another enum, that just has cases doNothing, remove

The enum definition has such cases already and the default state is to do nothing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants